Skip to content

Add Unix socket broker #125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 27, 2025
Merged

Add Unix socket broker #125

merged 4 commits into from
Mar 27, 2025

Conversation

AlanCoding
Copy link
Member

This is maybe, perhaps, a solution to #100 but it is debatable.

However, it was part of the vision of dispatcherctl described in #61

This was more complicated than I expected, but I got it working.

Using the "old" demo with 2 terminal tabs.

# tab 1
PYTHONPATH=. dispatcherd
# tab 2
./run_demo.py

It now get a bit awkward because you can no longer get a response from multiple servers. But we get to the end of the demo, and it prints everything. In my first pass it broke half-way through because the message was longer than 1024. I think I'm still putting in "reply_to" which the server just ignores.

Protocols need to be updated and basically everything aside from making it run the demo once, which is all it does right now.

@AlanCoding
Copy link
Member Author

Oh, here's an interesting find from the demo output:

  Task-234 is done=False
   trace:
Stack for <Task pending name='Task-234' coro=<Broker._add_client() running at /home/alancoding/repos/dispatcher/dispatcher/brokers/socket.py:45> wait_for=<Future pending cb=[Task.task_wakeup()]> cb=[StreamReaderProtocol.connection_made.<locals>.callback() at /usr/lib64/python3.12/asyncio/streams.py:248]> (most recent call last):
  File "/home/alancoding/repos/dispatcher/dispatcher/brokers/socket.py", line 45, in _add_client
    line = await client.reader.readline()

It's right about where we expected it to be.

@AlanCoding
Copy link
Member Author

Again, I hit an issue where it will be difficult to move forward before #121 lands.

When running commands as dispatcherctl entrypoint (the ultimate point of this), it can hang waiting for a reply. (yes this means timeout isn't working)

The reply isn't coming because origin information is 0. This is, predictably, because the control.py module has not had its contract updated to the same as DispatcherMain. However, the PR 121 removes the need to copy this contract in the first place. So that's where this leaves us.

@AlanCoding
Copy link
Member Author

I figured out what it was. In asyncio, there are problems having parallel tasks both listening and writing to the same connection. This is roughly the same issue I hit with pg_notify. The solution is same in form, except for the multiple tasks for different clients.

@AlanCoding AlanCoding marked this pull request as ready for review March 17, 2025 17:28
@AlanCoding AlanCoding changed the title [still-demo] Add Unix socket broker Add Unix socket broker Mar 17, 2025
@AlanCoding AlanCoding added the enhancement New feature or request label Mar 17, 2025
@AlanCoding AlanCoding force-pushed the socket branch 2 times, most recently from 0f6b41b to 2cb88f3 Compare March 19, 2025 03:05
@AlanCoding AlanCoding force-pushed the socket branch 2 times, most recently from 552c282 to 04b7f61 Compare March 24, 2025 13:41
Protocol the brokers

Run demo dispatcherctl over socket

Add socket broker unit tests
Add socket broker usage integration tests

Work out issues with test scope and server not opening client connections

Finish type hinting

Fix contract problem

Incremental JSON parse

Run linters
@AlanCoding
Copy link
Member Author

I vetted the basic functionality with AWX and it's working so I'm going ahead with merge. Some code organizational issues are documented, but this isn't meant to be polished at this point.

@AlanCoding AlanCoding merged commit 995455b into ansible:main Mar 27, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants